Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always hide controls if video is playing [Android TV] #3537

Closed
wants to merge 2 commits into from

Conversation

Stypox
Copy link
Member

@Stypox Stypox commented May 5, 2020

What is it?

  • Bug fix (user facing)
  • Feature (user facing)
  • Code base improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

In #2806 a new safeHideControls function was added, that would hide controls only if they were opened using a touch gesture. This is obviously never the case when using a tv, so after safeHideControls was used (i.e. after every click on the interface), nothing would happen, preventing controls from ever being hidden. I assume this was added because on a tv it takes some time to press the correct buttons, thus having the ui close while selecting things would have been annoying, so now I made it so that safeHideControls hides controls while the video is playing, too, and otherwise the old behaviour is preserved. Also see #2806 (comment)

Fixes the following issue(s)

Testing apk

@Poomex @B0pol @Generator could you test if the usability on TV devices is preserved, even if the ui is hidden when the video is playing?
app-debug.zip

Agreement

@Stypox Stypox added bug Issue is related to a bug player Issues related to any player (main, popup and background) Android TV Issue is related to Android TV labels May 5, 2020
@Stypox Stypox changed the title Always hide controls after a while if video is playing on Android TV Always hide controls if video is playing [Android TV] May 5, 2020
@B0pol
Copy link
Member

B0pol commented May 5, 2020

It's fixed for me. Though I've one concern: the speed is not the same as usual, i.e. SEVEN seconds
We definitely should standardize. But why it's so high on TV??? I would remove this and just let normal time on all other devices, which is 2 seconds (line just above).

It's used here and here.

Edit: here, there is no need for another reaction / click, so it's normal that the time is shorter.

@Stypox
Copy link
Member Author

Stypox commented May 6, 2020

d29e0aa is the commit which increased the delay, I guess for usability reasons (on some TVs maybe 2s is too short to be able to do anything?)

B0pol
B0pol previously approved these changes May 6, 2020
@TobiGr TobiGr added this to the 0.19.4 milestone May 6, 2020
@Stypox
Copy link
Member Author

Stypox commented May 6, 2020

@B0pol I reduced to 5s the hide time. New debug apk: app-debug.zip

@Alexander--
Copy link
Contributor

Alexander-- commented May 7, 2020

@B0pol

I would remove this and just let normal time on all other devices, which is 2 seconds

Sounds like you have lighting-fast reaction. Have you actually used NewPipe on TV?

Quickly looking over controls, reading all labels, and pressing the navigation buttons — all of that in under 2 seconds — is impressive feat (especially since infrared TV remotes have additional ~100 ms of lag due to limitatons of IR protocols).

@Stypox

Right now method safeHideControls is being used from onClick() only. Hiding controls in onClick() makes some sense on touch-based devices, because showing them again is cheap, and the user is probably done interacting with screen anyway. Hiding controls from onClick() does not make sense on TV devices, because re-summoning and navigating around them with dpad is a massive hassle.

On TV devices safeHideControls does nothing (unless they are in touch mode). It does not abort current timeout, if present. So if controls were going to be hidden, they will still get hidden.

If something (showControls/showControlsThenHide) does not hide controls properly on TV devices, that should be changed. Method safeHideControls currently works as designed — it should not hide controls any quicker just because user has clicked on one of them.

@Alexander--
Copy link
Contributor

@B0pol

Note, that 7 second timeout isn't really expected to be triggered. You are meant to dismiss controls with BACK button after you are done with them.

I agree, that media buttons (PLAY,PAUSE,REWIND etc.) should have different treatment.

@Stypox
Copy link
Member Author

Stypox commented May 7, 2020

Quickly looking over controls, reading all labels, and pressing the navigation buttons — all of that in under 2 seconds — is impressive feat (especially since infrared TV remotes have additional ~100 ms of lag due to limitatons of IR protocols).

Anyway, as soon as something is pressed, the player resets the hiding countdown

@Alexander--
Copy link
Contributor

@Stypox the point is — in some circumstances there is no cooldown. When controls are summoned by showControls (such as when playback is paused), they do not disappear. But if you click on something — for example, change aspect ratio — the controls quickly get hidden again. This makes no sense to me.

I believe, that controls should stay permanently summoned after playback is paused — to remind user that playback was indeed paused and not just hanged. Resuming from pause via media buttons — including the buttons on headset — should quickly hide controls, because headsets do not have BACK button to quickly dismiss them.

I have sent #3547 with those changes.

@Stypox Stypox modified the milestones: 0.19.4, 0.19.5 May 28, 2020
@B0pol
Copy link
Member

B0pol commented Jun 26, 2020

@Poomex please test the apk in the original post

@Poomex
Copy link

Poomex commented Jun 27, 2020

Unfortunately, the issue hasn't been fixed for me. Pressing play on my bluetooth speaker keeps the interface visible until I tap the screen.

@TobiGr TobiGr modified the milestones: 0.19.6, 0.20.0 Jul 7, 2020
@MD77MD
Copy link

MD77MD commented Jul 14, 2020

I>I believe, that controls should stay permanently summoned after playback is paused — to remind user that playback was indeed paused and not just hanged.

although sometimes showing controls is needed... I disagree with you in saying hiding them don't make sense, hiding player controls is also useful. sometimes a quick pause and play action is needed such as for reading a table or a chart, for comparing two pics or even for taking a screenshot because it keeps the controls out of the way.

my suggestion regarding showing controls after pause or hiding them is divided in two parts as fallow:

#For phones:
A. when to show controls after pause: when pause is triggered by tapping the pause button, that is:

  1. while video is playing
  2. tap screen to show controls
  3. tap pause button
  4. video is paused while showing controls

B. when to hide controls after pause: when using double tap to pause gesture. that is:

  1. while video is playing
  2. double tap to pause
  3. player controls are are kept hidden
  4. perform action the pause was intended for
    (look above for examples)
  5. double tap to play

#For TVs:
A. when to show controls after pause: when pause is triggered by pressing the ok button on the remote
B. when to hide controls after pause: when pause is triggered by pressing the play/pause button on the remote

This unifies the double tap to seek and pause/play gestures behavior related to player controls showing or hiding.. it also gives newPipe a cleaner UI feeling for both TVs and smartphones whenever a quick pause and play action is needed like what I explained above

@wb9688 wb9688 modified the milestones: 0.20.0, 0.20.1 Jul 29, 2020
@Stypox
Copy link
Member Author

Stypox commented Oct 4, 2020

@Alexander-- should I close this in favour of #3547?

@Alexander--
Copy link
Contributor

@Stypox

Yes, I suggest you do

@Stypox Stypox deleted the fix-tv-player branch October 4, 2020 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Android TV Issue is related to Android TV bug Issue is related to a bug player Issues related to any player (main, popup and background)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The player controls don't hide when using external controls
8 participants